fix(parser): parenthesized type as an arrow return type (#62) - #65
Merged
Conversation
`looksLikeFunctionTypeParams` ended with a "scan to the matching `)` then check
for `=>`" heuristic that classified ANY `( … ) =>` as a function type, ignoring
the content. So an arrow return type that is a parenthesized type —
`const f = (): (void) => {}` — was read as the function type `(void) => {}`,
swallowing the arrow's own `=>`; the whole arrow then became an ErrorNode.
`(void)`, `(T | U)`, `(readonly T[])` etc. all hit this.
Replace the scan-forward with TS's isUnambiguouslyStartOfFunctionType logic:
skip an optional parameter-property modifier and exactly ONE parameter binding
(identifier / `this` / destructuring pattern — NOT a reserved bare-type keyword
like `void`/`null`/`true`/`false`/`this`, and NOT a leading `(`), then require a
parameter separator (`:` `?` `=` `,`) or `)` immediately followed by `=>`. A
type operator after the first identifier (`T | U`, `T[]`) therefore correctly
falls through to a parenthesized type, leaving the `=>` for the enclosing arrow.
Validated: full suite green; TS conformance back at baseline 17910/17913 ·
1210/1223 (an initial over-strict version regressed 5 parameter-list/function-
type cases — `(public B) =>` modifiers and `({}?: T) =>` optional patterns —
now covered by the modifier-skip and the `?` separator); babel 1928/1928 ·
1548/1548; test262 3966/3966 · 1389/1389; semantic sweep 0 crashes with only
positive deltas (+8 scopes / +37 symbols / +28 refs — arrows recovered from
ErrorNodes), diagnostics unchanged.
…s)`, tests Three review findings on the looksLikeFunctionTypeParams rewrite: 1. Scope-event leak (regression review). Classifying expression-shaped parens like `(b = 1)` as function-type params makes the conditional-consequent typed-arrow speculation in parseParenthesized run parseFunctionType, which emits scope_open/declare events. The backtrack restored tok/nodes/extra but NOT the event stream, leaking a phantom function scope + parameter (wrong reference data for `c ? x : (b = 1) && b`). Add Parser.resetEventsTo and truncate events on both backtrack paths. 2. Memory leak: that same speculation also allocPrints a diagnostic (no `=>` after the fake params); the backtrack's shrinkRetainingCapacity dropped it without freeing the message. Add Parser.truncateDiagnostics (frees discarded messages, matching the tree's deinit) and use it on the backtrack paths. 3. `(this) =>` fidelity (correctness review): a bare `this` parameter is a valid function type in TS, so accept `.kw_this` as a parameter binding instead of treating `(this)` as a parenthesized type. Tests: pin the arrow's actual ArrowData.return_type (not a loose whole-tree hasNodeTag scan); add union/intersection/conditional return-type cases; add a semantic regression test that the conditional-with-paren-alternate leaks no phantom scope or duplicate parameter. Validated: full suite green (no leaks under the testing allocator); TS conformance 17910/17913 · 1210/1223; babel 1928/1928 · 1548/1548; test262 3966/3966 · 1389/1389.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #62.
Problem
looksLikeFunctionTypeParamsended with a "scan to the matching), then check for=>" heuristic that classified any( … ) =>as a function type, ignoring the content. So an arrow whose return type is a parenthesized type —— was read as the function type
(void) => {}, swallowing the arrow's own=>.(void),(T | U),(readonly T[]), etc. all hit this. (Parenthesized param types and bare return types already worked.)Fix
Replace the scan-forward with TS's
isUnambiguouslyStartOfFunctionTypelogic: skip an optional parameter-property modifier and exactly one parameter binding (identifier /this/ destructuring pattern — not a reserved bare-type keyword likevoid/null/true/false/this, and not a leading(), then require a parameter separator (:?=,) or)immediately followed by=>. A type operator after the first identifier (T | U,T[]) therefore falls through to a parenthesized type, leaving the=>for the enclosing arrow.Validation
(void),(T | U),(readonly T[]), nested, object-shorthand, js_ts) parse as arrows (no ErrorNode); the return type is aTSParenthesizedType; genuine function types still parse asTSFunctionType(including(public B) =>modifiers and({}?: T) =>optional patterns); a bare(void)stays a parenthesized type.(public B) =>and({}?: T) =>— now covered by the modifier-skip and the?separator.)